-
Notifications
You must be signed in to change notification settings - Fork 469
Update privileges for pcr in v25.2 #19605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for cockroachdb-api-docs canceled.
|
✅ Deploy Preview for cockroachdb-interactivetutorials-docs canceled.
|
✅ Netlify Preview
To edit notification comments on pull requests, go to your Netlify project configuration. |
e70c0e4
to
6189e9e
Compare
@@ -200,17 +200,6 @@ Connect to your standby cluster's system virtual cluster using [`cockroach sql`] | |||
--certs-dir "certs" | |||
~~~ | |||
|
|||
1. Add your cluster organization and [{{ site.data.products.enterprise }} license]({% link {{ page.version.version }}/licensing-faqs.md %}#types-of-licenses) to the cluster: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking this out because of the licensing change. (I should go back through to v24.3 and make sure to remove.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest pls go back and remove from v24.3 and v25.1 if at all possible :-)
6189e9e
to
e6125ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except I think we should remove all mention of the SET REPLICATION SOURCE EXPIRATION WINDOW = duration. I think this is too much information for the user and can result in unexpected behavior for the source cluster, which we also don't test. See https://cockroachlabs.slack.com/archives/CJXBR3693/p1747684142786379?thread_ts=1746716551.106479&cid=CJXBR3693
e6125ac
to
4582dee
Compare
@@ -58,15 +57,15 @@ You can use the following options with `ALTER VIRTUAL CLUSTER {vc} START REPLICA | |||
|
|||
Option | Value | Description | |||
-------+-------+------------ | |||
`EXPIRATION WINDOW` | Duration | Override the default producer job's expiration window of 24 hours. The producer job expiration window determines how long the producer job will continue to run without a heartbeat from the consumer job. For more details, refer to the [Technical Overview]({% link {{ page.version.version }}/physical-cluster-replication-technical-overview.md %}). | |||
`EXPIRATION WINDOW` | Duration | Override the default producer job's expiration window of 24 hours from the primary cluster. The producer job expiration window determines how long the producer job will continue to run without a heartbeat from the consumer job. For more details, refer to the [Technical Overview]({% link {{ page.version.version }}/physical-cluster-replication-technical-overview.md %}). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think expiration window and retention need to be removed.
@@ -40,8 +40,6 @@ Parameter | Description | |||
`PAUSE REPLICATION` | Pause the replication stream. | |||
`RESUME REPLICATION` | Resume the replication stream. | |||
`COMPLETE REPLICATION TO` | Set the time to complete the replication. Use: <br><ul><li>`SYSTEM TIME` to specify a [timestamp]({% link {{ page.version.version }}/as-of-system-time.md %}). Refer to [Fail over to a point in time]({% link {{ page.version.version }}/failover-replication.md %}#fail-over-to-a-point-in-time) for an example.</li><li>`LATEST` to specify the most recent replicated timestamp. Refer to [Fail over to a point in time]({% link {{ page.version.version }}/failover-replication.md %}#fail-over-to-the-most-recent-replicated-time) for an example.</li></ul> | |||
`SET REPLICATION RETENTION = duration` | Change the [duration]({% link {{ page.version.version }}/interval.md %}) of the retention window that will control how far in the past you can [fail over]({% link {{ page.version.version }}/failover-replication.md %}) to.<br><br>{% include {{ page.version.version }}/physical-replication/retention.md %} | |||
<span class="version-tag">New in v25.2:</span> `SET REPLICATION SOURCE EXPIRATION WINDOW = duration` | Override the default producer job's expiration window of 24 hours from the primary cluster. The producer job expiration window determines how long the producer job will continue to run without a heartbeat from the consumer job. For more details, refer to the [Technical Overview]({% link {{ page.version.version }}/physical-cluster-replication-technical-overview.md %}). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK to remove SET REPLICATION SOURCE EXPIRATION WINDOW
too? @msbutler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i think we want to remove that documentation too https://cockroachlabs.slack.com/archives/CJXBR3693/p1747688082011559?thread_ts=1746716551.106479&cid=CJXBR3693
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let some comments but otherwise looks good -- thank you!
@@ -41,8 +40,6 @@ Parameter | Description | |||
`PAUSE REPLICATION` | Pause the replication stream. | |||
`RESUME REPLICATION` | Resume the replication stream. | |||
`COMPLETE REPLICATION TO` | Set the time to complete the replication. Use: <br><ul><li>`SYSTEM TIME` to specify a [timestamp]({% link {{ page.version.version }}/as-of-system-time.md %}). Refer to [Fail over to a point in time]({% link {{ page.version.version }}/failover-replication.md %}#fail-over-to-a-point-in-time) for an example.</li><li>`LATEST` to specify the most recent replicated timestamp. Refer to [Fail over to a point in time]({% link {{ page.version.version }}/failover-replication.md %}#fail-over-to-the-most-recent-replicated-time) for an example.</li></ul> | |||
`SET REPLICATION RETENTION = duration` | Change the [duration]({% link {{ page.version.version }}/interval.md %}) of the retention window that will control how far in the past you can [fail over]({% link {{ page.version.version }}/failover-replication.md %}) to.<br><br>{% include {{ page.version.version }}/physical-replication/retention.md %} | |||
`SET REPLICATION EXPIRATION WINDOW = duration` | Override the default producer job's expiration window of 24 hours. The producer job expiration window determines how long the producer job will continue to run without a heartbeat from the consumer job. For more details, refer to the [Technical Overview]({% link {{ page.version.version }}/physical-cluster-replication-technical-overview.md %}). | |||
`START REPLICATION OF virtual_cluster_spec ON physical_cluster` | Reset a virtual cluster to the time when the virtual cluster on the promoted standby diverged from it. To reuse as much of the existing data on the original primary cluster as possible, you can run this statement as part of the [failback]({% link {{ page.version.version }}/failover-replication.md %}#failback) process. This command fails if the virtual cluster was not originally replicated from the original primary cluster. Refer to [Options](#options) for details on how you can configure a PCR stream initiated as a failback. | |||
`START SERVICE SHARED` | Start a virtual cluster so it is ready to accept SQL connections after failover. | |||
`RENAME TO virtual_cluster_spec` | Rename a virtual cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having trouble leaving a comment on the actual section which is 'Options' but I don't think I understand what that section means. Does that mean that if you have a reader cluster running you need to include that SQL syntax during the failover?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we didn't need to do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean ALTER VIRTUAL CLUSTER .... with READ VIRTUAL CLUSTER
? @msbutler I'm pretty sure confirmed with me that we should keep that in — sorry was trying to find the thread.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I just don't understand the use case here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we need to keep this option (with READER VIRTUAL CLUSTER or whatever it is) . It's used in the reader tenant upgrade flow. https://cockroachlabs.slack.com/archives/C06G041QAMC/p1746624798507089
`READ VIRTUAL CLUSTER` | N/A | ([**Preview**]({% link {{ page.version.version }}/cockroachdb-feature-availability.md %}#features-in-preview)) Configure the PCR stream to allow reads from the standby cluster. <br>**Note:** This only allows for reads on the standby's virtual cluster. You cannot perform writes or schema changes to user tables while connected to the standby virtual cluster. For more details, refer to [Start the failback process](#start-the-failback-process). | ||
`RETENTION` | Duration | Change the [duration]({% link {{ page.version.version }}/interval.md %}) of the retention window that will control how far in the past you can [fail over]({% link {{ page.version.version }}/failover-replication.md %}) to.<br><br>{% include {{ page.version.version }}/physical-replication/retention.md %} | ||
|
||
## Examples | ||
|
||
### Start the failover process | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the 'Start the failover process' should be combined with the 'start a virtual cluster,' or rather, the 'start a virtual cluster' part should be a subsection of the failover process section. This is because generally the 'failover' isn't complete until the standby cluster is ready to receive traffic. So I think we should put these two steps together into one 'Failover process' section.
@@ -24,7 +24,7 @@ The `DROP VIRTUAL CLUSTER` statement will delete all data related to the specifi | |||
`DROP VIRTUAL CLUSTER` requires one of the following privileges: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add "if a user wanted to completely restart their PCR stream, they would drop their old VC and create a new VC via a replication stream." some additional context on why one would do this?
|
||
When a virtual cluster is [`ready`]({% link {{ page.version.version }}/show-virtual-cluster.md %}#responses) after initiating the cutover process, you must start the service so that the virtual cluster is ready to accept SQL connections: | ||
To [cut back]({% link {{ page.version.version }}/cutover-replication.md %}#cutback) to a cluster that was previously the primary cluster, use the `ALTER VIRTUAL CLUSTER` syntax: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just to clarify, in 24.1 docs, what we normally call failback, we call cutback? Perhaps in a follow up PR we should make this consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree the consistency would be best across versions, but I think we "formally" switched over the name at that version and didn't backport the change for syntax reasons. I'll look back over communication here, and add to the list.
@@ -77,9 +67,9 @@ Value | Description | |||
Cockroach Labs does not recommend changing the default capabilities of created virtual clusters. | |||
{{site.data.alerts.end}} | |||
|
|||
_Capabilities_ control what a virtual cluster can do. When you start a replication stream, you can specify a virtual cluster with `LIKE` to ensure other virtual clusters on the standby cluster will work in the same way. `LIKE` will refer to a virtual cluster on the CockroachDB cluster you're running the statement from. | |||
_Capabilities_ control what a virtual cluster can do. The configuration profile included at startup creates the `template` virtual cluster with the same set of capabilities per CockroachDB version. When you start a replication stream, you can specify the `template` VC with `LIKE` to ensure other virtual clusters on the standby cluster will work in the same way. `LIKE` will refer to a virtual cluster on the CockroachDB cluster you're running the statement from. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be removed entirely right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to just tackle capabilities next week. https://cockroachlabs.atlassian.net/browse/DOC-13797
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to change that in this PR because that link was breaking the build.
|
||
- The `admin` role. | ||
- The `MANAGEVIRTUALCLUSTER` [system privilege]({% link {{ page.version.version }}/security-reference/authorization.md %}#privileges) allows the user to run all the related `VIRTUAL CLUSTER` SQL statements for PCR. | ||
{% include_cached new-in.html version="v25.2" %} The `ALTER VIRTUAL CLUSTER ... SET REPLICATION SOURCE` statement requires the `REPLICATIONSOURCE` system privilege and the `MANAGEVIRTUALCLUSTER` privilege. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be removed, since we're no longer documenting this cmd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed. I've made an issue for myself to clear up any last mentions of these, which can coincide with the updated diagrams as they merge from the cockroach
repo https://cockroachlabs.atlassian.net/browse/DOC-13800
@@ -41,8 +40,6 @@ Parameter | Description | |||
`PAUSE REPLICATION` | Pause the replication stream. | |||
`RESUME REPLICATION` | Resume the replication stream. | |||
`COMPLETE REPLICATION TO` | Set the time to complete the replication. Use: <br><ul><li>`SYSTEM TIME` to specify a [timestamp]({% link {{ page.version.version }}/as-of-system-time.md %}). Refer to [Fail over to a point in time]({% link {{ page.version.version }}/failover-replication.md %}#fail-over-to-a-point-in-time) for an example.</li><li>`LATEST` to specify the most recent replicated timestamp. Refer to [Fail over to a point in time]({% link {{ page.version.version }}/failover-replication.md %}#fail-over-to-the-most-recent-replicated-time) for an example.</li></ul> | |||
`SET REPLICATION RETENTION = duration` | Change the [duration]({% link {{ page.version.version }}/interval.md %}) of the retention window that will control how far in the past you can [fail over]({% link {{ page.version.version }}/failover-replication.md %}) to.<br><br>{% include {{ page.version.version }}/physical-replication/retention.md %} | |||
`SET REPLICATION EXPIRATION WINDOW = duration` | Override the default producer job's expiration window of 24 hours. The producer job expiration window determines how long the producer job will continue to run without a heartbeat from the consumer job. For more details, refer to the [Technical Overview]({% link {{ page.version.version }}/physical-cluster-replication-technical-overview.md %}). | |||
`START REPLICATION OF virtual_cluster_spec ON physical_cluster` | Reset a virtual cluster to the time when the virtual cluster on the promoted standby diverged from it. To reuse as much of the existing data on the original primary cluster as possible, you can run this statement as part of the [failback]({% link {{ page.version.version }}/failover-replication.md %}#failback) process. This command fails if the virtual cluster was not originally replicated from the original primary cluster. Refer to [Options](#options) for details on how you can configure a PCR stream initiated as a failback. | |||
`START SERVICE SHARED` | Start a virtual cluster so it is ready to accept SQL connections after failover. | |||
`RENAME TO virtual_cluster_spec` | Rename a virtual cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, we need to keep this option (with READER VIRTUAL CLUSTER or whatever it is) . It's used in the reader tenant upgrade flow. https://cockroachlabs.slack.com/archives/C06G041QAMC/p1746624798507089
@@ -77,9 +67,9 @@ Value | Description | |||
Cockroach Labs does not recommend changing the default capabilities of created virtual clusters. | |||
{{site.data.alerts.end}} | |||
|
|||
_Capabilities_ control what a virtual cluster can do. When you start a replication stream, you can specify a virtual cluster with `LIKE` to ensure other virtual clusters on the standby cluster will work in the same way. `LIKE` will refer to a virtual cluster on the CockroachDB cluster you're running the statement from. | |||
_Capabilities_ control what a virtual cluster can do. The configuration profile included at startup creates the `template` virtual cluster with the same set of capabilities per CockroachDB version. When you start a replication stream, you can specify the `template` VC with `LIKE` to ensure other virtual clusters on the standby cluster will work in the same way. `LIKE` will refer to a virtual cluster on the CockroachDB cluster you're running the statement from. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this can be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but see my comments re: retroactively removing features and there doesn't seem to be a DOC ticket saying to do that so we can find the provenance of this change in the future
@@ -66,19 +65,6 @@ You can use either: | |||
- `SYSTEM TIME` to specify a [timestamp]({% link {{ page.version.version }}/as-of-system-time.md %}). | |||
- `LATEST` to specify the most recent replicated timestamp. | |||
|
|||
### Set a retention window |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't see from either of the linked issues that they talk about removing these retention & expiration options, but i am probably not reading them right. if i am reading them right and maybe this is an opportunistic edit, is there a JIRA tracking this part of the work we can refer to in future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, apologies Rich, issue now linked. This was a last-minute decision that should have gone into a separate PR. There will also be follow-up work on this.
@@ -38,16 +38,8 @@ GRANT SYSTEM MANAGEVIRTUALCLUSTER TO user; | |||
Parameter | Description | |||
----------+------------ | |||
`virtual_cluster_name` | The name for the new virtual cluster. | |||
`LIKE virtual_cluster_spec` | Creates a virtual cluster with the same [capabilities](#capabilities) and settings as another virtual cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems weird that we are ripping features out of a very old release like this? maybe this is all happening via team discussions so apologies if so. just seems quite unusual to be removing stuff this far back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PS on further review, suggest checking if this has interactions with any 'known limitations' or 'backward-incompatible changes' we published in these earlier release docs
@@ -47,13 +47,20 @@ Parameter | Description | |||
|
|||
## Examples | |||
|
|||
### Remove a virtual cluster | |||
### Restart a PCR stream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
opportunistic edit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
|
||
|
||
The `DROP VIRTUAL CLUSTER` statement removes virtual clusters. Virtual clusters are used only as part of the [**physical cluster replication (PCR)**]({% link {{ page.version.version }}/physical-cluster-replication-overview.md %}) workflow. | ||
The `DROP VIRTUAL CLUSTER` statement removes virtual clusters in order to restart a **physical cluster replication (PCR)** stream. Virtual clusters are used only as part of the [PCR]({% link {{ page.version.version }}/physical-cluster-replication-overview.md %}) workflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
officially (*) we don't need to update v24.2 docs anymore
(*) except unofficially we do because google can still find stuff in them :-/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, though, I forgot, but maybe it's good to update them for these cases (particularly removal of options)
@@ -19,12 +19,12 @@ Physical cluster replication is only supported in CockroachDB {{ site.data.produ | |||
|
|||
## Required privileges | |||
|
|||
`CREATE VIRTUAL CLUSTER` requires one of the following privileges: | |||
{% include_cached new-in.html version="v25.2" %} The following [privileges]({% link {{ page.version.version }}/security-reference/authorization.md %}) are required to start a PCR stream with `CREATE VIRTUAL CLUSTER`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest adding #supported-privileges
to the link to save users a scroll
@@ -105,7 +105,7 @@ Connect to your primary cluster's system virtual cluster using [`cockroach sql`] | |||
|
|||
### Create a replication user and password | |||
|
|||
The standby cluster connects to the primary cluster's system virtual cluster using an identity with the `REPLICATION` privilege. Connect to the primary cluster's system virtual cluster and create a user with a password: | |||
The standby cluster connects to the primary cluster's system virtual cluster using an identity with the `REPLICATIONSOURCE` privilege. Connect to the primary cluster's system virtual cluster and create a user with a password: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest linking this use of "privilege" with the privileges docs
@@ -200,17 +200,6 @@ Connect to your standby cluster's system virtual cluster using [`cockroach sql`] | |||
--certs-dir "certs" | |||
~~~ | |||
|
|||
1. Add your cluster organization and [{{ site.data.products.enterprise }} license]({% link {{ page.version.version }}/licensing-faqs.md %}#types-of-licenses) to the cluster: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest pls go back and remove from v24.3 and v25.1 if at all possible :-)
2aa99e0
to
59b0719
Compare
Fixes DOC-12796, DOC-13026, DOC-13814
Updates the privileges for PCR SQL statements for 25.2.
And removed the retention, expiration options.